feat(windows): prefer PowerShell defaults for shell tools#16069
feat(windows): prefer PowerShell defaults for shell tools#16069Hona wants to merge 10 commits intoanomalyco:devfrom
Conversation
Use pwsh and powershell before Git Bash when SHELL is unset on Windows so the default shell matches native expectations more closely. Surface the active OS and shell in the bash tool definition so agents can reason about the runtime they are executing in.
There was a problem hiding this comment.
Pull request overview
Adjusts Windows shell selection defaults to prefer PowerShell-family shells and improves runtime transparency for the bash tool by surfacing OS/shell context in its description.
Changes:
- Update
Shell.preferred()/Shell.acceptable()to preferpwsh, thenpowershell, whenSHELLis unset on Windows. - Keep existing Windows Git Bash path-resolution fallback behavior intact.
- Inject
process.platformand a resolved shell name into thebashtool description template.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/opencode/src/tool/bash.txt | Adds OS + resolved shell placeholders to the tool description. |
| packages/opencode/src/tool/bash.ts | Resolves shell “display name” and substitutes OS/shell into the tool description. |
| packages/opencode/src/shell/shell.ts | Introduces base() helper and changes Windows default shell preference order. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (process.platform === "win32") { | ||
| if (Bun.which("pwsh")) return "pwsh" | ||
| if (Bun.which("powershell")) return "powershell" | ||
| } | ||
| return fallback() |
There was a problem hiding this comment.
On Windows, returning the bare string "pwsh"/"powershell" here can break downstream logic that assumes shells ending with "sh" are POSIX shells (e.g. Pty.create() appends -l when command.endsWith("sh"), which matches pwsh). Consider returning the resolved executable path from Bun.which(...) (typically ends with .exe) or otherwise ensuring the returned value won’t match that heuristic; also consider de-duplicating the pwsh/powershell selection logic between preferred and acceptable to avoid drift.
packages/opencode/src/shell/shell.ts
Outdated
| if (process.platform === "win32") return path.win32.basename(file, ".exe") | ||
| return path.basename(file) |
There was a problem hiding this comment.
base() is used for the blacklist check, but on Windows path.win32.basename(file, ".exe") only strips the extension if the casing matches exactly, and it preserves casing of the name. If SHELL is something like Nu.EXE or NU.EXE, base() won’t produce "nu", so the blacklist won’t apply. Consider using path.win32.parse(file).name.toLowerCase() (or lowercasing the basename result) to make the check robust.
| if (process.platform === "win32") return path.win32.basename(file, ".exe") | |
| return path.basename(file) | |
| if (process.platform === "win32") { | |
| // Use parse().name to strip the extension in a case-insensitive way, then normalize to lowercase | |
| return path.win32.parse(file).name.toLowerCase() | |
| } | |
| // On non-Windows, just take the basename and normalize to lowercase | |
| return path.basename(file).toLowerCase() |
| export const preferred = lazy(() => { | ||
| const s = process.env.SHELL | ||
| if (s) return s | ||
| if (process.platform === "win32") { | ||
| if (Bun.which("pwsh")) return "pwsh" | ||
| if (Bun.which("powershell")) return "powershell" | ||
| } | ||
| return fallback() | ||
| }) | ||
|
|
||
| export const acceptable = lazy(() => { | ||
| const s = process.env.SHELL | ||
| if (s && !BLACKLIST.has(process.platform === "win32" ? path.win32.basename(s) : path.basename(s))) return s | ||
| if (s && !BLACKLIST.has(base(s))) return s | ||
| if (process.platform === "win32") { | ||
| if (Bun.which("pwsh")) return "pwsh" | ||
| if (Bun.which("powershell")) return "powershell" | ||
| } |
There was a problem hiding this comment.
This PR changes Windows shell selection behavior (pwsh/powershell preference and updated blacklist logic), but there are no unit tests covering Shell.preferred() / Shell.acceptable() on win32 or the interaction with call sites like Pty.create(). Adding targeted tests (with platform/env/which stubbing) would help prevent regressions like incorrect args being appended or blacklisted shells slipping through.
Remove the one-off helper used for shell name normalization and keep the Windows blacklist check inline. This keeps the shell selection logic simpler without changing behavior.
Hona
changed the title
Summary
pwsh, thenpowershell, before Git Bash andcmd.exeon Windows whenSHELLis unset for bothShell.preferred()andShell.acceptable()bashlookupprocess.platformand resolved shell name in thebashtool definition so agents can see the runtime context